Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support dots and slashes in virtual custom attributes #14329

Merged
merged 5 commits into from
Mar 16, 2017

Conversation

lpichler
Copy link
Contributor

@lpichler lpichler commented Mar 14, 2017

temporarily replacing #11112
fixes #10482 point 1
Using URI::RFC2396_Parser from #13713 - thanks! @gtanzillo @cben @zeari

@miq-bot add_label euwe/yes, bug, reporting

cc @simon3z

please review @cben @zeari

@miq-bot assign @gtanzillo

I have explanation in commit messages.
tested:

  • creating/updating UI reports with these attributes
  • using filtering with MiqExpression with virtual custom attributes in report

Links [Optional]

Scenarios:

  • add any custom attributes with special characters for any Vm
  • create report based for example on Virtual Machines
  • select those custom attributes and add it to report
    Case1
    Generate report and see results
    Case2
    create expression with using custom attributes
    Generate report and see results
    Case3
    as Case2 but remove custom attribute from selected field and leave it in expression
    Generate report and see results

How to add custom attributes

Vm.all.each do |vm|
  $evm = MiqAeMethodService::MiqAeService.new(MiqAeEngine::MiqAeWorkspaceRuntime.new)
  vm = $evm.vmdb('vm', vm.id )
  vm.custom_set("ATTRfsdfafame_1_#{vm.id}",  nil)
end

@lpichler lpichler changed the title support dots and slashes in CustomAttribute Support dots and slashes in virtual custom attributes Mar 14, 2017
@lpichler
Copy link
Contributor Author

@miq-bot remove_label euwe/yes (it needs special PR for euwe)

@miq-bot
Copy link
Member

miq-bot commented Mar 14, 2017

@lpichler Cannot remove the following label because they are not recognized: euwe/yes (it needs special pr for euwe)

special characters in custom attributes
Field with virtual custom attributes  don’t contains associations so it don’t contain “.” as separator

it has always form:

<Model>-virtual_custom_attribute_<name_of_attribute>

After this  we need to escape it back because there was dynamic method created with unescaped form,
and such method will be called in evaluation.
before:
ns= ‘/virtual/parent_resource_pool/name’
predicate = ns.split("/")
=> ["", "virtual", "parent_resource_pool", "name"]
predicate = ns.split(“/“)[2..-1] # throw away /virtual
=> ["parent_resource_pool", "name"]

after: (same)
ns= ‘/virtual/parent_resource_pool/name’
ns.gsub!("/virtual/","")  # throw away /virtual
=> ‘parent_resource_pool/name’
predicate = ns.split("/")
=> ["parent_resource_pool", "name"]
@lpichler lpichler changed the title Support dots and slashes in virtual custom attributes [WIP] Support dots and slashes in virtual custom attributes Mar 16, 2017
@miq-bot miq-bot added the wip label Mar 16, 2017
@lpichler
Copy link
Contributor Author

euwe pr #14363

…acters

now there is allowed to have special characters as `-` and `.` `/`
@lpichler lpichler changed the title [WIP] Support dots and slashes in virtual custom attributes Support dots and slashes in virtual custom attributes Mar 16, 2017
@miq-bot
Copy link
Member

miq-bot commented Mar 16, 2017

Checked commits lpichler/manageiq@9e09e03~...faaecaf with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
4 files checked, 5 offenses detected

app/models/tag.rb

lib/miq_expression.rb

@miq-bot miq-bot removed the wip label Mar 16, 2017
Copy link
Member

@gtanzillo gtanzillo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

ns.gsub!('/virtual/','') # throw away /virtual
ns, virtual_custom_attribute = MiqExpression.escape_virtual_custom_attribute(ns)
predicate = ns.split("/")
predicate.map!{ |x| URI::RFC2396_Parser.new.unescape(x) } if virtual_custom_attribute
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a suggestion, perhaps you can create an unescape_virtual_custom_attribute too. Can be done in a followup PR.

@gtanzillo gtanzillo added this to the Sprint 57 Ending Mar 27, 2017 milestone Mar 16, 2017
@gtanzillo gtanzillo merged commit a9ca06a into ManageIQ:master Mar 16, 2017
@simaishi
Copy link
Contributor

Backported to Euwe via #14363

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

custom attributes break report editor if name contains '.'
4 participants